Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix: raise error when get invalid consensus params #1790

Closed
wants to merge 6 commits into from
Closed

fix: raise error when get invalid consensus params #1790

wants to merge 6 commits into from

Conversation

mmsqe
Copy link
Contributor

@mmsqe mmsqe commented Jun 15, 2023

Closes: #XXX

Description

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@mmsqe mmsqe marked this pull request as ready for review June 15, 2023 03:40
@mmsqe mmsqe requested a review from a team as a code owner June 15, 2023 03:40
@mmsqe mmsqe requested review from Vvaradinov and MalteHerrmann and removed request for a team June 15, 2023 03:40
@mmsqe mmsqe mentioned this pull request Jun 15, 2023
Merged
11 tasks
@codecov
Copy link

codecov bot commented Jun 15, 2023 •
edited
Loading

Codecov Report

Merging #1790 (da74c62) into main (10d9579) will not change coverage.
The diff coverage is 100.00%.

❗ Current head da74c62 differs from pull request most recent head 444b8e8. Consider uploading reports for the commit 444b8e8 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1790   +/-   ##
=======================================
  Coverage   68.89%   68.89%           
=======================================
  Files         117      117           
  Lines       10681    10681           
=======================================
  Hits         7359     7359           
  Misses       2900     2900           
  Partials      422      422
Impacted Files Coverage Δ
x/feemarket/keeper/eip1559.go 90.16% <100.00%> (ø)

mmsqe added a commit to crypto-org-chain/ethermint that referenced this pull request Jun 15, 2023
@mmsqe
fix: add nil check for consParams.Block (backport: evmos#1790) (#279)
d14eada 
* add nil check for consParams.Block

* add change log

* skip invalid consensus params

* panic on invalid consensus params
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 15, 2023
View reviewed changes
x/feemarket/keeper/eip1559.go
if consParams != nil && consParams.Block.MaxGas > -1 {
gasLimit = big.NewInt(consParams.Block.MaxGas)
if consParams == nil || consParams.Block == nil || consParams.Block.MaxGas <= -1 {
panic(fmt.Sprintf("get invalid consensus params: %s", consParams))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
@mmsqe mmsqe changed the title fix: add nil check for consParams.Block fix: raise error when get invalid consensus params Jun 15, 2023
mmsqe added a commit to mmsqe/ethermint that referenced this pull request Jun 22, 2023
@mmsqe
Revert "fix: add nil check for consParams.Block (backport: evmos#1790) (
c43a018 
#279)"

This reverts commit d14eada.
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

@github-actions github-actions bot closed this Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Vvaradinov Vvaradinov Awaiting requested review from Vvaradinov Vvaradinov is a code owner automatically assigned from evmos/core-engineering

@MalteHerrmann MalteHerrmann Awaiting requested review from MalteHerrmann MalteHerrmann is a code owner automatically assigned from evmos/core-engineering

Assignees
No one assigned
Labels
Status: Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@mmsqe